Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Yosys42 #2646

Merged
merged 33 commits into from
Aug 23, 2024
Merged

Yosys42 #2646

merged 33 commits into from
Aug 23, 2024

Conversation

amirarjmand93
Copy link
Contributor

@amirarjmand93 amirarjmand93 commented Jul 15, 2024 •
edited
Loading

Description

This update brings Yosys to version 0.42, which includes several bug fixes, performance improvements, and new features that enhance the synthesis capabilities of our flow. Due to incompatibility with the latest Yosys, F4PGA plug-in related tests and tasks have been disabled in the flow until a new plugin is installed.
According to the QoR results, there is a significant improvement in time and area from synthesis to routing in most tests and architectures.

Related Issue

  • Upgrade to Yosys v42 (previous version was 0.32)

#- {test: "vtr_reg_system_verilog", cores: "16", options: "", cmake: "-DYOSYS_F4PGA_PLUGINS=ON",
- {test: "parmys_reg_strong", cores: "16", options: "", cmake: "-DYOSYS_F4PGA_PLUGINS=OFF",

  • F4PGA:After turning off DYOSYS_F4PGA_PLUGINS, some errors and failures appeared, but they have been resolved.
  • SV tasks:System Verilog tasks have been disabled. (parmys_reg_strong has been kept without the build of F4PGA )
  • Golden Result:Some tests were failing randomly due to machine load and very small fractional values (e.g. 10 times faster in pack_time) in comparison with regnerated golden result. As a result, pass_requirements.txt was modified, and some metrics' absolute thresholds were increased(from 2 to 3) to ensure the tests pass.

Important Features Added to Yosys from Versions 32 to 42

General Enhancements

  • "$print" Cell: Introduced for Verilog tasks "$display" and "$write", with handling in CXXRTL.
  • Lattice FPGA Support:
    • Generic "synth_lattice" pass for MachXO2/XO3/XO3D.
    • Removed "synth_machxo2"; replaced with "synth_lattice -family xo2".
    • Renamed "ecp5_gsr" to "lattice_gsr".
  • QuickLogic Support:
    • Added "K6N10f" support.
    • New options for "synth_quicklogic": "-nodsp", "-nocarry", "-nobram", "-bramtypes".
    • New passes: "ql_bram_merge", "ql_bram_types", "ql_dsp_io_regs", "ql_dsp_macc", "ql_dsp_simd".
  • Verific Support:
    • Improved handling of attributes, module parameters, memory inference, and mixed language projects.
    • Better support for VHDL 2009 and constants.

New Commands and Options

  • "sim" Pass: Options "-assert" and "-noinitstate".
  • "abc" Pass: Option "-dont_use".
  • "synth" Pass: Option "-extra-map".
  • "dfflibmap" Pass: Option "-dont_use".
  • "show" Command: Option "-href".
  • "flatten" Pass: Options "-noscopeinfo" and "-scopename".
  • "read_verilog" Command: Option "-nodisplay".
  • "verific" Pass: Option "-vhdl2019".
  • "opt_lut" Pass: Option "-tech".
  • "async2sync" and "clk2fflogic" Passes: Option "-nolower".
  • "chformal" Pass: Option "-lower".
  • "cellmatch" Pass: New command for picking out standard cells.

Optimization and Synthesis Enhancements

  • "peepopt" Pass: Enhanced shiftmul matcher and added shiftadd pattern support.
  • "booth" Pass: Map $mul cells to Booth multipliers; enabled by default for MachXO3 in "synth_lattice".
  • abc9: Enabled by default for ECP5, iCE40, and Gowin; option "-noabc9" to disable.
  • Incremental Mode: Added "--incremental" mode to smtbmc.

Advanced Features

  • $scopeinfo: Cells to preserve hierarchy information during flattening.
  • CXXRTL: Generic record/replay interface, capturing $print cell output.
  • smtbmc: Incremental mode.
  • Combinational Loop Detection: Improved in write_aiger.

Intel Support

  • Dropped Quartus support in "synth_intel_alm" pass.

Types of changes

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

@github-actions github-actions bot added lang-cpp C/C++ code infra Project Infrastructure lang-python Python code lang-make CMake/Make code lang-hdl Hardware Description Language (Verilog/VHDL) lang-shell Shell scripts (bash etc.) Parmys labels Jul 15, 2024
@amirarjmand93
Copy link
Contributor Author

@vaughnbetz I'm testing the final commits, but the CI test seems to be stuck and never succeeds. Also, "pushing an empty commit" or "closing and reopening the PR" didn't work. I switched to a new branch(this branch), but the problem persists.

@amirarjmand93 amirarjmand93 changed the title Yosys42 [WIP] Yosys42 Jul 15, 2024
@vaughnbetz
Copy link
Contributor

Not sure what the problem is. Perhaps CI has gotten overloaded again? (if so, try later tonight and it should work). @AlexandreSinger : any ideas?

@AlexandreSinger AlexandreSinger changed the title [WIP] Yosys42 Yosys42 Jul 15, 2024
@AlexandreSinger
Copy link
Contributor

Not sure what the problem is. Perhaps CI has gotten overloaded again? (if so, try later tonight and it should work). @AlexandreSinger : any ideas?

Based on the fact that the tests are saying "Expected - Waitiing for status to be reported"; I think that the CI is refusing to run this PR since the title says "WIP". I removed WIP from the title, but it still seems to be stalled.

I noticed that this branch has conflicts with .github/workflows/test.yml. This is the script that dictates what tests to run on the CI. I have a suspicion that this may be the culprit. Please try to rebase this branch onto master and resolve the conflict.

@amirarjmand93
Copy link
Contributor Author

amirarjmand93 commented Jul 15, 2024 •
edited
Loading

I had changed the test.yml(disable system verilog test) but I was able to continue with .github/workflows/test.yml conflict warning successfully last time( all tests ran). but this time I am getting this issue.

@AlexandreSinger
Copy link
Contributor

I had changed the test.yml(disable system verilog test) but I was able to continue with .github/workflows/test.yml conflict warning successfully last time( all tests ran). but this time I am getting this issue.

I am not sure then what could be the root cause; but that (along with the WIP status) were the only differences between this PR and other that are able to run the CI.

Regardless the conflicts need to be resolved before the code can be merged.

@amirarjmand93 amirarjmand93 force-pushed the Yosys42 branch 2 times, most recently from ba0d1e9 to 273d7b2 Compare July 16, 2024 01:22
@AlexandreSinger
Copy link
Contributor

@amirarjmand93 It looks like resolving the conflicts seem to have launched the CI. The vtr_reg_nightly failures seems to be an infrastructure issue on VTR's end. The self-hosted CI runners are not responding. This is happening on other runs; I am looking into it.

However, the "R: Basic ..." tests seem to be issue with your PR:
image
It looks like some QoR failures that need to have their golden results updated I think.

@amirarjmand93 amirarjmand93 force-pushed the Yosys42 branch 2 times, most recently from f839aab to dad6580 Compare July 17, 2024 01:35
@amirarjmand93
Copy link
Contributor Author

@amirarjmand93 It looks like resolving the conflicts seem to have launched the CI. The vtr_reg_nightly failures seems to be an infrastructure issue on VTR's end. The self-hosted CI runners are not responding. This is happening on other runs; I am looking into it.

However, the "R: Basic ..." tests seem to be issue with your PR: image It looks like some QoR failures that need to have their golden results updated I think.

all VTR and Parmys golden results are committed.
F4PGA is off and System Verilog tasks are disabled.
but I think the runner is still down.

@vaughnbetz
Copy link
Contributor

Yes, the self hosted runners are down ... we'll discuss tomorrow but perhaps they have been deactivated in which case we'll have to come up with a new strategy.

@AlexandreSinger
Copy link
Contributor

The CI is back up. I have re-launched the CI on this PR. The PR is having build issues with Yosys:
image

There seems to be an unresolved conflict in the Makefile of Yosys:
image

@AlexandreSinger
Copy link
Contributor

I cancelled the CI since there is a build issue that needs to be resolved.

@amirarjmand93
Copy link
Contributor Author

Thanks, @vaughnbetz and @AlexandreSinger.
Today I missed the meeting. these days we have been encountering frequent outages.
I'll look at it.

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool libarchfpga Library for handling FPGA Architecture descriptions docs Documentation libvtrutil labels Jul 19, 2024
@vaughnbetz
Copy link
Contributor

Nice detective work @amirarjmand93 and @AlexandreSinger !
With the routing failure predictor off, a minimum channel width run could have extra chaotic (and long) runtime behaviour. I agree; let's take mcml out of this test. If LU32PE is also taking a long time in this test we could take it out too. This is really a test to make sure the router works without the predictor on and gets a reasonable minimum channel width. It isn't that important a test and will scale very poorly (and somewhat chaotically) with circuit size so we should avoid having large circuits in it.

@AlexandreSinger AlexandreSinger mentioned this pull request Aug 19, 2024
Closed
@AlexandreSinger
Copy link
Contributor

@amirarjmand93 Your last commit disabled MCML and LU32 in vtr_reg_qor_chain, which was not the testcase which was timing out (which is why the CI still timed out). We need to disable these circuits in vtr_reg_nightly_test3/vtr_reg_qor_chain_predictor_off.

@amirarjmand93
Copy link
Contributor Author

@vaughnbetz Nightly1 is causing problems again due to small fractional values. by local PC, all tests pass.
The absolute threshold changed from 3 to 5 but it was not effective.
In some tests, it seems the Golden result should be halved to pass!
e.x. fixed_k6_frac_N8_22nm.xml/mult_092.v/common min_chan_width_route_time relative value 0.056373017389642656 outside of range [0.1,15.0], above absolute threshold 5.0 and not equal to golden value: 52.33

Probably, we should reconsider the pass_requirement file(Run_time metrics):

image

@vaughnbetz
Copy link
Contributor

I'm fine with cutting the lower limit for min_chan_width route time to 0.05. I suggest going ahead and making that change.

@amirarjmand93
Copy link
Contributor Author

Finally done :)

@vaughnbetz
Copy link
Contributor

CI is green! Yaay! Thanks for driving this @amirarjmand93 !

@AlexandreSinger : I think you wanted to do a QoR run to check the parsing scripts & get a full comparison (VTR benchmarks and some of the Koios ones I think). If you can get that soon I can wait and then merge. I'd like to get this in fairly soon though ...

@AlexandreSinger
Copy link
Contributor

@vaughnbetz Yes, that is correct! I will do that this evening! We should hopefully be able to land this by tomorrow morning. I'll add some commits if there is something that needs to be added.

@AlexandreSinger
[Yosys] Cleaned Up Some Small Issues
71ffc6f 
The changes made to Yosys added a few random lines which I removed.

Added some comments explaining why certain tests were turned off.

Upgraded libcatch to match master since the PR would have downgraded
master by mistake.
@AlexandreSinger
Merge remote-tracking branch 'origin/master' into Yosys42
78352e7
@AlexandreSinger
Copy link
Contributor

AlexandreSinger commented Aug 23, 2024 •
edited
Loading

@amirarjmand93 I made some small changes. Mainly just removing some extra lines, adding some comments on why the tests were turned off, and upgrading the libcatch module (since it was going to overwrite master if that was not upgraded properly).

@vaughnbetz I did a QoR comparison for MCNC (to get some quick results) and the comparison looked mostly fine to me (no data was missing that the old parse_script was not also missing). I will run VTR tomorrow, since it will take a few hours to run, but I am confident that any problems can be fixed-up in another PR.

Great job on this PR @amirarjmand93 ; thank you so much for the hard work!

@amirarjmand93
Copy link
Contributor Author

amirarjmand93 commented Aug 23, 2024 •
edited
Loading

I dropped some comments for that but without much explanation.
Also, I brought some descriptions for the new Yosys update in the first comment in this PR.
Actually, I work alone in the Lab and this is my first experience with the VTR flow. I wouldn't have found success without that rallying behind me. Thanks @vaughnbetz and @AlexandreSinger.

@AlexandreSinger
Copy link
Contributor

@vaughnbetz This LGTM. I'll run the VTR tests this morning for a QoR test to try and see if the scripts have any issues; but I think this can be merged in the meantime.

@AlexandreSinger
Copy link
Contributor

AlexandreSinger commented Aug 23, 2024 •
edited
Loading

@vaughnbetz The VTR results are complete. The compare QoR script worked exactly as expected. Here is the summary:

  vtr_parse_results_old_yosys.txt vtr_parse_results_new_yosys.txt
vtr_flow_elapsed_time 1 1.088049
odin_synth_time    
parmys_synth_time 1 1.358482
abc_depth 1 0.985913
abc_synth_time 1 1.005816
num_clb 1 1.004224
num_memories 1 1
num_mult 1 1
max_vpr_mem 1 1.017276
num_pre_packed_blocks 1 0.999332
num_post_packed_blocks 1 1.001586
device_grid_tiles 1 1.004878
pack_time 1 0.946503
placed_wirelength_est 1 1.001315
place_time 1 0.960784
placed_CPD_est 1 1.010059
min_chan_width 1 0.974115
routed_wirelength 1 1.016237
min_chan_width_route_time 1 1.127555
crit_path_routed_wirelength 1 1.002678
critical_path_delay 1 0.99498
geomean_nonvirtual_intradomain_critical_path_delay 1 0.99498
crit_path_route_time 1 0.99919

Here is the data per circuit:

circuit vtr_flow_elapsed_time routed_wirelength critical_path_delay min_chan_width_route_time
arm_core.v 1.17832725 1.03698355 0.98473018 1.49657152
bgm.v 1.08138473 1.01923929 0.95139658 1.14864613
blob_merge.v 1.11661957 1.04982715 0.97739277 1.22924028
boundtop.v 1.24415714 0.93845018 1.05938204 1.13
ch_intrinsics.v 0.66966292 1.24366312 1.00364986 0.34972678
diffeq1.v 0.84358047 0.97858782 1.00745381 0.69858713
diffeq2.v 1.79666012 1.15473285 1.03724599 2.36040609
mkDelayWorker32B.v 1.06309567 0.97136452 1.07587452 0.98004561
mkPktMerge.v 1.01861702 0.93335093 0.97525595 0.99803922
mkSMAdapter4B.v 1.24819734 1.03536387 0.97468448 1.62787724
or1200.v 0.8862852 0.93849267 0.93373149 0.69794721
raygentop.v 1.13616071 1.01624624 0.95639283 1.25957973
sha.v 0.80302241 1.0005925 1.02911415 0.48061105
spree.v 0.90139907 0.96399579 0.98084684 0.56756757
stereovision0.v 1.01355225 1.06564465 0.99326977 0.96230916
stereovision1.v 0.81292805 1.03377766 0.9691305 0.64301149
stereovision2.v 1.06767848 1.02937026 1.00271384 1.17549087
stereovision3.v 1.09689923 1 1 1.05555556
LU8PEEng.v 0.97605767 0.97374592 0.98408924 0.86490165
LU32PEEng.v 2.7245183 1.03082152 0.99942957 8.97119005
mcml.v 1.29522906 0.9750117 1.01049827 3.97223211
GEOMEAN 1.08804895 1.01623685 0.99498048 1.12755536

I noticed that LU32PEEng took 3x longer than the old Yosys version; however, this is due to the minimum channel width route time being 9x slower. I did check, the pack place and route times (non-min channel width) are all within reason.

I think this PR is good to go.

Here is the raw data in case you were curious:
vtr_old_v_new.xlsx

@vaughnbetz
Copy link
Contributor

Excellent! Thanks @amirarjmand93 and @AlexandreSinger !

@vaughnbetz vaughnbetz merged commit 8f62ad3 into master Aug 23, 2024
52 checks passed
@vaughnbetz vaughnbetz deleted the Yosys42 branch August 23, 2024 17:05
@AlexandreSinger AlexandreSinger mentioned this pull request Aug 27, 2024
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexandreSinger AlexandreSinger AlexandreSinger left review comments

Assignees
No one assigned
Labels
docs Documentation external_libs infra Project Infrastructure lang-cpp C/C++ code lang-hdl Hardware Description Language (Verilog/VHDL) lang-make CMake/Make code lang-python Python code lang-shell Shell scripts (bash etc.) libarchfpga Library for handling FPGA Architecture descriptions libvtrutil Parmys VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@amirarjmand93 @vaughnbetz @AlexandreSinger